test(mcp): reject control-padded submit_work_proof format (#819) by yanyishuai · Pull Request #1189 · ramimbo/mergework

yanyishuai · 2026-06-30T08:11:57Z

Summary

Adds focused regression coverage for #819: submit_work_proof must reject raw control-padded format values such as \u0085json instead of normalizing them to json.

Changes

tests/test_api_mcp.py — assert /mcp tools/call returns the existing invalid tool arguments envelope for control-padded format input

Verification

pytest tests/test_api_mcp.py::test_mcp_submit_work_proof_rejects_control_padded_format -q
pytest tests/test_api_mcp.py -q

Fixes #819

Wallet: Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

Summary by CodeRabbit

Tests
- Added coverage for an MCP API request that uses a control-padded format value.
- Verified the API returns the expected invalid tool arguments response for this malformed input.

coderabbitai · 2026-06-30T08:12:15Z

📝 Walkthrough

Walkthrough

Adds one test, test_mcp_submit_work_proof_rejects_control_padded_format, that posts a submit_work_proof MCP tool call with format: "\x85json" and asserts the response matches the -32602 invalid tool arguments error envelope.

Changes

Control-padded format rejection test

Layer / File(s)	Summary
Regression test for control-padded format `tests/test_api_mcp.py`	New test creates schema, calls `submit_work_proof` with `format="\x85json"`, asserts HTTP 200, and verifies the invalid tool arguments envelope via `_assert_invalid_tool_arguments_envelope`.

Possibly related PRs

ramimbo/mergework#839: Adds the production-side control-character guard in output_format_arg() that this test exercises.
ramimbo/mergework#317: Introduced format: "json" structured output and initial invalid format checks for submit_work_proof.
ramimbo/mergework#856: Tightened submit_work_proof argument/schema validation, directly related to the boundary this test covers.

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR only adds regression coverage; it does not implement the required control-character guard for `#819`.	Add the `contains_control_character(value)` guard in `output_format_arg()` before `strip().lower()`, while keeping the regression test.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is short, concrete, and matches the changed MCP test surface.
Description check	✅ Passed	It includes a summary, changes, verification commands, and an issue reference, so it is mostly aligned with the template.
Out of Scope Changes check	✅ Passed	Only a focused test was added in `tests/test_api_mcp.py`, which matches the stated MCP regression scope.
Mergework Public Artifact Hygiene	✅ Passed	Only a test file was added; no README/docs/public-comment text with investment, price, cash-out, or private-security claims was introduced.
Bounty Pr Focus	✅ Passed	The PR is scoped to tests/test_api_mcp.py and adds a focused submit_work_proof format-rejection regression; I found no unrelated surfaces in the change.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c8bca4c6-35ac-41e0-92fa-27562739cd55

📥 Commits

Reviewing files that changed from the base of the PR and between 3bc87d2 and 4b41819.

📒 Files selected for processing (1)

tests/test_api_mcp.py

coderabbitai · 2026-06-30T08:16:44Z

+    assert response.status_code == 200
+    _assert_invalid_tool_arguments_envelope(response.json(), request_id=37)


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the field-level validation payload too.

Line 2485 only checks the legacy -32602 envelope, so this test still passes if submit_work_proof fails for some other invalid-arguments path. Please also assert the safe error.data payload for tool="submit_work_proof", field="format", and the control-character message so the regression stays tied to this guard.

Suggested tightening

assert response.status_code == 200 - _assert_invalid_tool_arguments_envelope(response.json(), request_id=37) + response_json = response.json() + _assert_invalid_tool_arguments_envelope(response_json, request_id=37) + error = response_json["error"] + assert isinstance(error, dict) + data = error["data"] + assert isinstance(data, dict) + assert data["tool"] == "submit_work_proof" + assert data["field"] == "format" + assert data["message"] == "format must not contain control characters"

As per coding guidelines, "Add or update tests for changed behavior." As per path instructions, "Focus on whether tests prove the changed behavior and include negative, replay, boundary, or regression cases where relevant."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

assert response.status_code == 200

_assert_invalid_tool_arguments_envelope(response.json(), request_id=37)

assert response.status_code == 200

response_json = response.json()

_assert_invalid_tool_arguments_envelope(response_json, request_id=37)

error = response_json["error"]

assert isinstance(error, dict)

data = error["data"]

assert isinstance(data, dict)

assert data["tool"] == "submit_work_proof"

assert data["field"] == "format"

assert data["message"] == "format must not contain control characters"

Sources: Coding guidelines, Path instructions

yanyishuai · 2026-06-30T08:24:46Z

@qingfeng312 This is a focused regression test for #819 — rejects control-padded submit_work_proof format values (\u0085json) via the existing invalid tool arguments envelope.

CI is green on the latest head and this is the only open PR for the issue. Merge-ready whenever maintainers have bandwidth.

Wallet: Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

qingfeng312

Reviewed current head 4b41819784f9b5fb28ab3094e9c3950077d93655.

Approved. This is a focused regression for the submit_work_proof format boundary: the new test sends a raw control-padded format value and verifies the existing invalid tool arguments envelope. Hosted quality checks pass on the exact head, so the current runtime path already rejects this input. The change is tests-only and does not touch ledger, payout, wallet, treasury, or public artifact behavior.

yanyishuai · 2026-06-30T10:21:02Z

@qingfeng312 Follow-up on #819 — the control-padded submit_work_proof format regression test is still green on 4b41819784f9 with your prior approval.

Only open PR for the issue; ready to merge when convenient.

Wallet: Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

yanyishuai · 2026-07-01T04:33:41Z

@qingfeng312 Follow-up on #819 — the control-padded submit_work_proof format regression test is still green on 4b41819784f9 with your prior approval.

Only open PR for the issue; ready to merge when convenient.

Wallet: Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

yanyishuai · 2026-07-01T08:36:29Z

@qingfeng312 Follow-up on #819 — the control-padded submit_work_proof format regression test is still green on 4b41819784f9 with your prior approval.

Only open PR for the issue; ready to merge when convenient.

Wallet: Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

yanyishuai · 2026-07-01T08:42:57Z

@qingfeng312 This is a focused regression test for #819 — rejects control-padded submit_work_proof format values (\u0085json) via the existing invalid tool arguments envelope.

CI is green on the latest head and this is the only open PR for the issue. Merge-ready whenever maintainers have bandwidth.

Wallet: Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

yanyishuai · 2026-07-01T08:56:12Z

@qingfeng312 Follow-up on #819 — the control-padded submit_work_proof format regression test is still green on 4b41819784f9 with your prior approval.

Only open PR for the issue; ready to merge when convenient.

Wallet: Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

yanyishuai · 2026-07-01T09:07:21Z

@qingfeng312 This is a focused regression test for #819 — rejects control-padded submit_work_proof format values (\u0085json) via the existing invalid tool arguments envelope.

CI is green on the latest head and this is the only open PR for the issue. Merge-ready whenever maintainers have bandwidth.

Wallet: Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

yanyishuai · 2026-07-01T09:18:20Z

@qingfeng312 Follow-up on #819 — the control-padded submit_work_proof format regression test is still green on 4b41819784f9 with your prior approval.

Only open PR for the issue; ready to merge when convenient.

Wallet: Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

yanyishuai · 2026-07-01T09:44:56Z

@qingfeng312 Follow-up on #819 — the control-padded submit_work_proof format regression test is still green on 4b41819784f9 with your prior approval.

Only open PR for the issue; ready to merge when convenient.

Wallet: Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

test(mcp): reject control-padded submit_work_proof format (ramimbo#819)

4b41819

coderabbitai Bot reviewed Jun 30, 2026

View reviewed changes

qingfeng312 approved these changes Jun 30, 2026

View reviewed changes

qingfeng312 mentioned this pull request Jun 30, 2026

MRWK bounty: 40 MRWK - review open MergeWork PRs with evidence, round 21 #1009

Open

		assert response.status_code == 200
		_assert_invalid_tool_arguments_envelope(response.json(), request_id=37)

-    assert response.status_code == 200
-    _assert_invalid_tool_arguments_envelope(response.json(), request_id=37)
+    assert response.status_code == 200
+    response_json = response.json()
+    _assert_invalid_tool_arguments_envelope(response_json, request_id=37)
+    error = response_json["error"]
+    assert isinstance(error, dict)
+    data = error["data"]
+    assert isinstance(data, dict)
+    assert data["tool"] == "submit_work_proof"
+    assert data["field"] == "format"
+    assert data["message"] == "format must not contain control characters"

Uh oh!

Conversation

yanyishuai commented Jun 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

yanyishuai commented Jun 30, 2026

Uh oh!

qingfeng312 left a comment

Choose a reason for hiding this comment

Uh oh!

yanyishuai commented Jun 30, 2026

Uh oh!

yanyishuai commented Jul 1, 2026

Uh oh!

yanyishuai commented Jul 1, 2026

Uh oh!

yanyishuai commented Jul 1, 2026

Uh oh!

yanyishuai commented Jul 1, 2026

Uh oh!

yanyishuai commented Jul 1, 2026

Uh oh!

yanyishuai commented Jul 1, 2026

Uh oh!

yanyishuai commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yanyishuai commented Jun 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 30, 2026 •

edited

Loading